make library_column_order a shared parent class as a move toward consistency#1671
make library_column_order a shared parent class as a move toward consistency#1671gerrycampion merged 6 commits intomainfrom
Conversation
| variables_metadata = [ | ||
| var | ||
| for var in variables_metadata | ||
| if var.get(self.params.key_name) == self.params.key_value |
There was a problem hiding this comment.
Previously for here is core was missing the code would fall back to "PERMISSIBLE" but here it would become None. Is this behavior change intended?
There was a problem hiding this comment.
This is the reason for the updates in sdtm_utilities. Does that resolve your concern?
| return list(OrderedDict.fromkeys(variable_names_list)) | ||
| # Filter variables based on the specified criteria | ||
|
|
||
| if self.params.key_name and self.params.key_value: |
There was a problem hiding this comment.
This might not be a big issue for our case but if any chance key-value be 0 or False then this comparison will not work as intended. Could the better approach be: if self.params.key_name is not None and self.params.key_value: is not None ?
If you think these values are not possible here we can skip the change.
There was a problem hiding this comment.
The values can be an empty string. I changed to only check truthiness of key_name. This way, if key_name is a non-empty string, we can do the filter. If key_value is missing, it will be treated as a filter for empty string.
RamilCDISC
left a comment
There was a problem hiding this comment.
The PR refactors the code to put shared code in parent class "library_column_order" to make code more consistent.
The PR was validated by:
- Review the PR for any unwanted code or comments.
- Review the PR in accordance with AC.
- Ensure all unit and regression testing pass.
- Ensure all relevant testing is updated.
- Ensure CORE_test_suite pass as this validates no rules are affected
- Manually run validation using dev editor.
- Ensure all downstream files are updated to use parent class.
made library_column_order a shared parent class for expected_variables, permissible_variables, required_variables, and get_dataset_filtered_variables. Achieved by adding optional filter params for library_column_order.
This is to reduce inconsistent handling of variable_metadata and wildcard replacement.